Skip to content

Added some fault-tolerance to the build queue #746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 7, 2020

Conversation

Kixiron
Copy link
Member

@Kixiron Kixiron commented May 4, 2020

Closes #628

Stops losing crates that are being added to the build queue when a crash occurs. Attempted to make add_crate_to_queue batched, but couldn't figure out how to with the current db driver

@Nemo157
Copy link
Member

Nemo157 commented May 4, 2020

I just noticed the

add_crate_to_queue(&conn, &krate.name, &krate.version, priority).ok();
debug!("{}-{} added into build queue", krate.name, krate.version);

hidden in there, maybe this should be changed to

match add_crate_to_queue(&conn, &krate.name, &krate.version, priority) {
  Ok(_) => debug!("{}-{} added into build queue", krate.name, krate.version),
  Err(err) => error!("failed adding {}-{} into build queue: {}", krate.name, krate.version, err),
}

@Kixiron
Copy link
Member Author

Kixiron commented May 4, 2020

I realized that we could also add a prometheus hook for this, if it's something we think is important enough

@jyn514
Copy link
Member

jyn514 commented May 4, 2020

I realized that we could also add a prometheus hook for this, if it's something we think is important enough

I don't this is necessary, we already track the number of crates queued and we can look at the logs or /releases/queue if we want to know the exact crates.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for working on this! Do you have any ideas on how to test this? I thought maybe we could delete the crate_priorities table in the test so that getting the connection and fetching the index succeed, but adding the crate fails.

@pietroalbini
Copy link
Member

Why did you change the permissions of the docker entrypoint?

@Kixiron
Copy link
Member Author

Kixiron commented May 7, 2020

Fixed, something with Windows messed up

@jyn514
Copy link
Member

jyn514 commented May 7, 2020

Thanks for working on this!

@jyn514 jyn514 merged commit 6060a0b into rust-lang:master May 7, 2020
@Kixiron Kixiron deleted the git-peekin branch May 7, 2020 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crates that are fetched but not added to the index because of a crash will be lost
4 participants